Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Input Model Refactor #302

Closed
wants to merge 9 commits into from
Closed

Input Model Refactor #302

wants to merge 9 commits into from

Conversation

MarkintoshZ
Copy link
Collaborator

@MarkintoshZ MarkintoshZ commented Jan 11, 2024 •
edited
Loading

PR for #296

  • Schema matching based on Kubernetes OpenAPI specification
    • Download OpenAPI specification by Kubernetes version
    • Schema matching based on matching sub-properties
    • Choosing the from multiple matching schema candidates based on edit distance between schema path suffix and schema name
    • Narrow down schemas candidates based on property names observed in OpenAPI Specification
    • Test cases
  • Generating test cases
    • Implementing generator decorator
    • Refactor old testcase generation code to new design
    • Validating new system generates the same testcases as the old one
    • Port a few old operators to the new system

@MarkintoshZ
Implement K8sSchemaMatcher class
2193e33
@MarkintoshZ
Apply property-name-based heuristic on ranking matched schema
ecc604e
@MarkintoshZ
Improve schema matching heuristic
560f586 
- Observe property names used for each k8s schemas in k8s schema spec
  and use them to improve ranking of matched schemas
@MarkintoshZ
Fix code style and add documentation
4f5523b
@MarkintoshZ
Add test cases for K8sSchemaMatcher
640a2fe 
- Added kafka_crd to test_data/
- Added test case generation code
- Partially fixed pylint warnings in test code
@MarkintoshZ
Match descendants of matched schemas
2df2fb7
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 11, 2024 •
edited
Loading

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)		Lines missing
  acto/input
  generator.py
  k8s_schemas.py 76-88, 98, 105, 115, 125, 132, 135, 157, 160, 179, 185, 188, 195, 198, 205, 208, 216, 304-305, 313, 319, 323, 405, 414-457
Project Total  

This report was generated by python-coverage-comment-action

@tylergu
Copy link
Member

tylergu commented Jan 11, 2024

I personally prefer the name "KubernetesMapSchema" and "KubernetesArraySchema" over "KubernetesDictSchema" and "KubernetesListSchema". As Dict and List are Python terms and Array is a JSON term.
Other than this, this patch looks great!

@tylergu
Copy link
Member

tylergu commented Jan 12, 2024

@MarkintoshZ , just curious, does the dump_schema method guarantee to persist the original schema's constraints? Just looking at the ObjectSchema it seems it only handles the schemas for each subproperty

@MarkintoshZ
Copy link
Collaborator Author

@tylergu It does not currently. In my vision, the testcase generator functions would be provided with (if available) the matched k8s schema object, which stores the Kubernetes schema spec in the schema_spec attribute. So those constraints would be available when generating test cases.

On second thought, do we still need to dump schema if each schema file is just part of the OpenAPI schema downloaded from Kubernetes' repo?

@MarkintoshZ
Use YAML for dumped schema files
4a9281c
@MarkintoshZ
Added generator decorator and tests for it
1d3c829 
- generator decorator supports k8s schema name, field name, field type,
  paths, and priority configuration
@MarkintoshZ MarkintoshZ mentioned this pull request Jan 16, 2024
Merged
@MarkintoshZ
Copy link
Collaborator Author

Close this PR because it is broken into #307 and #311

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MarkintoshZ @tylergu